fix(input): improve error text accessibility #30635
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const hasNgTouched = this.el.classList.contains('ng-touched'); | ||
| const hasNgInvalid = this.el.classList.contains('ng-invalid'); |
There was a problem hiding this comment.
We shouldn't have Angular checks in core, Angular should be adding ion-touched and ion-invalid at the same time. If we need to adjust this we should do it in Angular.
| const hasNgTouched = this.el.classList.contains('ng-touched'); | ||
| const hasNgInvalid = this.el.classList.contains('ng-invalid'); |
There was a problem hiding this comment.
We shouldn't have Angular checks in core, Angular should be adding ion-touched and ion-invalid at the same time. If we need to adjust this we should do it in Angular.
| </div> | ||
|
|
||
| <!-- Hidden region for debugging screen reader announcements --> | ||
| <div aria-live="polite" aria-atomic="true" class="aria-live-region" id="debug-region"></div> |
There was a problem hiding this comment.
What is the purpose of this? It seems to cause double announcements in VoiceOver:
With <div>
input-with-hidden-div.mov
Without <div>
input-without-hidden-div.mov
We should remove this as the tests should act as they would in an app without any extra aria elements. It seems to work fine without it.
| </div> | ||
|
|
||
| <!-- Hidden region for debugging screen reader announcements --> | ||
| <div aria-live="polite" aria-atomic="true" class="aria-live-region" id="debug-region"></div> |
There was a problem hiding this comment.
What is the purpose of this? It seems to cause double announcements in VoiceOver:
With <div>
with-hidden-div.mov
Without <div>
without-div.mov
We should remove this as the tests should act as they would in an app without any extra aria elements. It seems to work fine without it.
| this.isInvalid = newIsInvalid; | ||
| forceUpdate(this); | ||
| } | ||
| }, 100); |
There was a problem hiding this comment.
Is 100 needed here? Does no number work?
| // Update aria-live region if invalid | ||
| if (this.isInvalid(fieldName) && this.debugRegion) { | ||
| const metadata = this.fieldMetadata[fieldName as keyof typeof this.fieldMetadata]; | ||
| this.debugRegion.nativeElement.textContent = |
There was a problem hiding this comment.
Can we remove all debugging code?
| // Update aria-live region if invalid | ||
| if (this.isInvalid(fieldName) && this.debugRegion) { | ||
| const metadata = this.fieldMetadata[fieldName as keyof typeof this.fieldMetadata]; | ||
| this.debugRegion.nativeElement.textContent = |
There was a problem hiding this comment.
Can we remove all debugging code?
| if (this.isInvalid(fieldName)) { | ||
| element.classList.remove('ion-valid'); | ||
| element.classList.add('ion-invalid'); | ||
| } else if (this.isValid(fieldName)) { | ||
| element.classList.remove('ion-invalid'); | ||
| element.classList.add('ion-valid'); |
There was a problem hiding this comment.
We should not need to manually add or remove these classes.
| // Update ion-valid/ion-invalid classes | ||
| if (this.isInvalid(fieldName)) { | ||
| element.classList.remove('ion-valid'); | ||
| element.classList.add('ion-invalid'); | ||
| } else if (this.isValid(fieldName)) { | ||
| element.classList.remove('ion-invalid'); | ||
| element.classList.add('ion-valid'); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
We should not need to manually add or remove these classes.
| [class.ion-touched]="isTouched('email')" | ||
| [class.ion-invalid]="isInvalid('email')" | ||
| [class.ion-valid]="isValid('email')" |
There was a problem hiding this comment.
I suggest following how we implement the Input value accessor instead of manually adding these classes.
Issue number: resolves internal
What is the current behavior?
Currently, when an error text is shown, it may not announce itself to voice assistants. This is because the way error text currently works is by always existing in the DOM, but being hidden when there is no error. When the error state changes, the error text is shown, but as far as the voice assistant can tell it's always been there and nothing has changed.
What is the new behavior?
With these changes, both input and textarea have been updated so they'll properly announce error text when it shows up. We had to do this with a mutation observer and state because it's important in some frameworks, like Angular, that state changes to cause a re-render. This, combined with some minor aria changes, makes it so that when a field is declared invalid, it immediately announces the invalid state instead of waiting for the user to go back to the invalid field.
Does this introduce a breaking change?
Other information
Current dev build:
Screens
Textarea
Input